Conversation
…engine with placeholder replacement
…nts and study documents
dennis-zyska
left a comment
dennis-zyska
left a comment
There was a problem hiding this comment.
I reviewed the first half of the code, but I have an appointment now, so I will submit the review and come back to it later.
backend/db/migrations/20251227183831-create-template-placeholder-mapping.js
Outdated
Show resolved
Hide resolved
| * @param {Object} options.transaction | ||
| * @returns {Promise<Array<string>>} | ||
| */ | ||
| async getLanguages(data, options) { |
There was a problem hiding this comment.
if we retrieve all templates in the frontend, aren't there also other languages retrieved or only the language that is currently selected? because otherwise we already have the languages in the frontend right?
There was a problem hiding this comment.
When we load templates in the frontend, we only retrieve the template rows, not their template_content rows. The per-language contents lives in the template_content table and are fetched separately, so we do not already have all languages in the frontend.
There was a problem hiding this comment.
What I was a bit confused, you basically created another Editor here, but then integraded it in the original Editor? I think there are two ways, either you make the selection one higher level in the component Template.vue where you used the Editor or even better you integrate it well in the existing editor to reduce duplicate code. I'm find with the first solution for now to get it merged, but please have a look in the Editor.vue component and check what changes are really necessary or which can be moved to Template.vue
…ore and using numeric templateId
…e update before navigate
dennis-zyska
left a comment
dennis-zyska
left a comment
There was a problem hiding this comment.
Thanks for the revision, looks good, I think there is just a minor bug, once fixed we can merge it.
| isHtml: true | ||
| }; | ||
| const fallback = await getEmailFallbackContent(fallbackKey, context); | ||
| return { subject: fallback.subject, body: resolvedHtml, isHtml: true }; |
There was a problem hiding this comment.
does the resolvedHtml must be fallback.body?
There was a problem hiding this comment.
The body here should stay as resolvedHtml. Since fallback.body is the plain text disk fallback for when no template is used. Where as this uses the DB template for the HTML body and only uses the fallback file for the subject.
3 participants
No description provided.